Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CFGWalker to generate consolidated exit blocks #6079

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Conversation

tlively
Copy link
Member

@tlively tlively commented Nov 3, 2023

Previously CFGWalker designated a particular block as the "exit" block, but it
was just the block that happened to appear at the end of the function that
returned values by implicitly flowing them out. That exit block was not tied in
any way to other blocks that might end in returns, so analyses that needed to
perform some action at the end of the function would have had to perform that
action at the end of the designated exit block but also separately at any return
instruction.

Update CFGWalker to make the exit block a synthetic empty block that is a
successor of all other blocks that implicitly or explicitly return from the
function in case there are multiple such blocks, or to make the exit block the
single returning block if there is only one. This means that analyses will only
perform their end-of-function actions at the end of the exit block rather than
additionally at every return instruction.

Previously CFGWalker designated a particular block as the "exit" block, but it
was just the block that happened to appear at the end of the function that
returned values by implicitly flowing them out. That exit block was not tied in
any way to other blocks that might end in returns, so analyses that needed to
perform some action at the end of the function would have had to perform that
action at the end of the designated exit block but also separately at any return
instruction.

Update CFGWalker to make the exit block a synthetic empty block that is a
successor of all other blocks tthat implicitly or explicitly return from the
function in case there are multiple such blocks, or to make the exit block the
single returning block if there is only one. This means that analyses will only
perform their end-of-function actions at the end of the exit block rather than
additionally at every return instruction.
@tlively tlively requested review from ashleynh and kripken November 3, 2023 00:03
@tlively
Copy link
Member Author

tlively commented Nov 3, 2023

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this will not regress performance, but it might be worth doing some measurements just to be safe.

// The exit block for the function: either the single block that returns or
// flows values out of the function, or an empty synthetic block that is a
// successor of all such blocks. This block may not exist if a function
// traps or infinitely loops and therefore never exits.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth mentioning throwing here, which is not considered an "exit". That is, the exit is the "return to caller, possibly with a value" block, not a general "exit from function somehow" block (which would have included exceptions).

@tlively
Copy link
Member Author

tlively commented Nov 6, 2023

I confirmed that this does not change performance on --merge-locals, which uses local graph.

@tlively tlively merged commit ba2ebea into main Nov 6, 2023
27 checks passed
@tlively tlively deleted the cfg-exit-blocks branch November 6, 2023 23:43
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
Previously CFGWalker designated a particular block as the "exit" block, but it
was just the block that happened to appear at the end of the function that
returned values by implicitly flowing them out. That exit block was not tied in
any way to other blocks that might end in returns, so analyses that needed to
perform some action at the end of the function would have had to perform that
action at the end of the designated exit block but also separately at any return
instruction.

Update CFGWalker to make the exit block a synthetic empty block that is a
successor of all other blocks tthat implicitly or explicitly return from the
function in case there are multiple such blocks, or to make the exit block the
single returning block if there is only one. This means that analyses will only
perform their end-of-function actions at the end of the exit block rather than
additionally at every return instruction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants